Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX #12355

XLTechie · 2021-04-30T09:06:46Z

Link to issue number:

Fixes #10799
Fixes #12345
Fixes #12351
Replaces #12349

Summary of the issue:

The COM Registration Fixing Tool still showed its completion dialog if it failed, or if the user said no to the UAC dialog (If the UAC is cancelled, the COM Reg Fixing Tool still claims to have completed successfully, though an error is logged #12345).
The initial dialog didn't have a visual close button (red X), and was not responsive to the Escape or Alt+F4 keys (Run COM Registration fixing tool Dialog can't be closed by ALT+F4 or ESC #10799).
It didn't report errors to the user as pointed out by @CyrilleB79 in Refactored the COM Registration Fixing Tool, to make it fail more gracefully, and improve its UX #12349, and
Its initial warning dialog was not serving users well with its strong warning and presentation (COM Registration Fixing Tool shouldn't start with a warning dialog and off-putting language #12351).

User facing changes:

Initial dialog given a more friendly message, with a more beginner-themed explanation.
User may now press Escape/Alt+F4 to leave the dialog, as well as hitting Cancel.
There is now a visual close control (red X).
F1 context help is now available.
Errors are reported to the user by error code.
The tool no longer reports success if it fails.
The tool no longer reports success if the process is cancelled by UAC.

Development details:

Refactored this code to be in line with current coding standards, fix the above issues, and to give the tool a better over all UX.
Catch the Windows error generated on UAC cancel, and log that the process was stopped at the UAC.
Show other Windows errors to the user via a dialog, instead of just in the log.
Switched from using YES and NO buttons, to Continue and CANCEL.
Added a docstring. Linted and added more logging.
In order to implement the Continue and Cancel paradigm, created gui.nvdaControls._ContinueCancelDialog.

Testing strategy:

Tested all buttons by tabbing and pressing, and by keyboard shortcuts.
Tested that all windows appear when and where they should.
Confirmed that the "completed" message no longer appears on failure or cancellation.
Simulated a Windows error to prove that a message would appear to show the error to the user.

Known issues with pull request:

Upon UAC cancel, or upon successful completion, focus is moved from desktop or where ever it was, to the NVDA systray icon. That seems a little strange to me, although it is not new behavior. gui.postPopup is supposed to fix this, but it doesn't.
Even after being deleted, the progress window's speech appears to hang around until after the completion window is cleared, although this speech seems to be cancelled or interrupted. Again, not new behavior. Log fragment showing this below:

IO - speech.speak (02:07:07.616) - MainThread (8368):
Speaking ['COM Registration Fixing Tool', 'dialog', 'The COM Registration Fixing Tool has completed successfully.', CancellableSpeech (still valid)]
IO - speech.speak (02:07:07.621) - MainThread (8368):
Speaking ['OK', 'button', CancellableSpeech (still valid)]
IO - inputCore.InputManager.executeGesture (02:07:09.326) - winInputHook (6992):
Input: kb(desktop):enter
IO - speech.speak (02:07:09.406) - MainThread (8368):
Speaking ['COM Registration Fixing Tool', 'dialog', "Please wait while NVDA attempts to fix your system's COM registrations...", CancellableSpeech (still valid)]
IO - speech.speak (02:07:09.411) - MainThread (8368):
Speaking ['COM Registration Fixing Tool', CancellableSpeech (still valid)]

Code Review Checklist:

Summary by CodeRabbit

New Features
- Introduced a custom dialog with "Continue" and "Cancel" buttons for better user interaction.
User Experience Improvements
- Updated messaging in the COM Registration Fixing Tool for better clarity and guidance.
- Enhanced error handling and logging for a more reliable user experience.
- Users can now exit the initial window using the escape key or alt+f4.

michaelDCurran · 2021-07-12T21:43:41Z

What is the status of this pr now that pr #12560 has been merged? Has this one been replaced, or is there extra work in this pr?

XLTechie · 2022-03-15T10:08:33Z

@michaelDCurran, to answer your old question: There is extra work in this PR. #12560 did the back-end. This one does the front-end (UX), and picks up errors that are not currently being handled or shown to the user. However I have been unavailable for several months, and this has fallen enough behind master that it can no longer merge/rebase as-is. I am working to bring it up to a compilable state, and then finish anything left to go on it. I think it will be tested and good to go for 2022.3.

LeonarddeR · 2023-08-25T16:43:03Z

@XLTechie any chance you can continue this any time soon?

AppVeyorBot · 2023-09-27T11:20:52Z

PASS: Translation comments check.
FAIL: Unit tests. See test results for more information.
FAIL: Lint check. See test results for more information.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/h0ga1sxagrfdb9jr/artifacts/output/nvda_snapshot_pr12355-29445,054795cb.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.8,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 23.8,
TESTSETUP_START 0.0,
TESTSETUP_END 0.2,
TEST_START 0.0,
TEST_END 0.7,
FINISH_END 0.1

See test results for failed build of commit 054795cbe5

CyrilleB79 · 2024-01-25T14:04:38Z

Ping @XLTechie:
Do you plan to push forward this PR soon?

More specifically, I was about to implement a fix for #12345 when I remembered of your PR.

CyrilleB79 · 2024-01-26T08:46:36Z

For issues with compile setup, feel free to ask on nvda-devel mailing list where such issues are usually discussed and addressed.

AppVeyorBot · 2024-04-12T12:04:34Z

FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/kxhhr2g7x6tp2gsj/artifacts/output/nvda_snapshot_pr12355-31588,b7653daa.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.1,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 30.0,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 2.4,
FINISH_END 0.2

See test results for failed build of commit b7653daab8

seanbudd · 2024-09-02T00:03:52Z

@XLTechie - do you plan to finish this work? Otherwise I think it should be closed

XLTechie · 2024-09-02T05:42:02Z

@seanbudd yes. I put it aside because I kept running into wx issues with text formatting being very strange. I'll look at it again, and if I can't make it work as-is, I'll split it into separate PRs addressing the major portions, so the important bits can get merged. Sorry for letting it lay for so long--time flies.

seanbudd · 2024-09-02T05:48:35Z

No problem - draft PRs are totally fine - we just want to make sure theyre not fully abandoned

gui.onRunCOMRegistrationFixesCommand: - No longer shows success dialog on failure, or when the UAC is canceled or declined (nvaccess#12345).. - We now catch the Windows error that signals user cancel of UAC, and return as if "NO" had been initially chosen. - If there is a Windows error other than cancel of UAC, alert the user and show the error in a dialog. - Rewrote initial dialog message to be more friendly and helpful, and removed warning icon (nvaccess#12351). - Switched from using YES and NO buttons, to OK and Cancel (CANCEL button provides visual closure and escape key use). - Added a docstring and fully linted code. Added more debug logging.

user_docs/en/userGuide.md

user_docs/en/changes.md

user_docs/en/userGuide.md

source/gui/__init__.py

Co-authored-by: Sascha Cowley <[email protected]>

XLTechie · 2024-09-19T01:48:48Z

@SaschaCowley I have addressed your latest review. I'm not sure I like the user guide reordered as you requested, but I do agree with your reason for wanting to do it. In the end, all of the same information is there, which ever order it is presented in, so I'm fine with whatever.

source/gui/__init__.py

SaschaCowley · 2024-09-19T02:12:25Z

@XLTechie I agree it really doesn't read very well now. Perhaps we could rewrite it to be a little less technical and more to the point of what users are likely to care about? What do you think of something like this?

Installing and uninstalling programs on a computer can sometimes cause problems with the Windows registry that result in NVDA behaving abnormally.
This can happen, for example, after installing and uninstalling Adobe Reader, Math Player and other programs.
It can also happen after Windows updates, or during other events that access the registry.
THE COM Registration Fixing Tool attempts to fix these issues by repairing accessibility entries in the registry.

The types of issues this tool can fix include:

* NVDA reporting "unknown" or "pane" when navigating in browsers such as Firefox or Edge, mail programs such as Thunderbird, Windows Explorer, the task bar, and other programs.
* NVDA failing to switch between focus mode and browse mode when you expect it to.
* NVDA being very slow when navigating in browsers while using browse mode.

Because this tool corrects entries in the Windows registry, it requires administrative access to work, just like when installing a program.
If you have UAC (User Access Control) enabled, as most users do, you will need to follow whatever prompts are presented by UAC to run the tool successfully.

…y be used instead. This reverts commit 78a1398.

XLTechie · 2024-09-19T07:27:58Z

@SaschaCowley How about this? Not quite what you wrote, but closer. @Qchristensen you might want to get in on this user guide discussion.

Qchristensen

Updated user guide and dialog intro text reads well. Great work everyone

XLTechie mentioned this pull request Jun 9, 2021

COM Registration Fixing tool doesn't work #9039

Closed

lukaszgo1 mentioned this pull request Nov 3, 2021

Implement the gui.messageBox API #13007

Closed

XLTechie changed the title ~~Refactored the COM Registration Fixing Tool code to remove bugs and for better UX~~ Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX Mar 24, 2022

Adriani90 mentioned this pull request Mar 1, 2023

Firefox 57 and Later Unusable with NVDA #7778

Closed

This comment was marked as outdated.

Sign in to view

XLTechie force-pushed the CRFTRefactor branch from 5fb12ae to cb0466e Compare September 27, 2023 10:54

This comment was marked as outdated.

Sign in to view

XLTechie force-pushed the CRFTRefactor branch 2 times, most recently from 65efc5c to becd99a Compare April 12, 2024 11:28

seanbudd added conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. merge-early Merge Early in a developer cycle labels Jun 17, 2024

Luke Davis added 2 commits September 2, 2024 19:55

changes

239810d

XLTechie force-pushed the CRFTRefactor branch from becd99a to 239810d Compare September 2, 2024 23:55

pre-commit-ci bot and others added 3 commits September 2, 2024 23:56

Pre-commit auto-fix

5467ff6

String reformatting

a293607

Remove old imports

5dfea2a

XLTechie force-pushed the CRFTRefactor branch from a37cbb5 to 5dfea2a Compare September 3, 2024 00:15

XLTechie marked this pull request as ready for review September 3, 2024 01:06

XLTechie requested a review from a team as a code owner September 3, 2024 01:06

XLTechie commented Sep 18, 2024

View reviewed changes

user_docs/en/userGuide.md Outdated Show resolved Hide resolved

Correction to grammar clarification in user guide.

70b596c

SaschaCowley requested review from SaschaCowley and Qchristensen September 19, 2024 00:26

seanbudd approved these changes Sep 19, 2024

View reviewed changes

SaschaCowley reviewed Sep 19, 2024

View reviewed changes

Luke Davis added 2 commits September 18, 2024 21:23

Remove attribution from changes entries

5fd2344

Merge branch 'master' into CRFTRefactor

4e88743

XLTechie commented Sep 19, 2024

View reviewed changes

source/gui/__init__.py Outdated Show resolved Hide resolved

XLTechie commented Sep 19, 2024

View reviewed changes

source/gui/__init__.py Outdated Show resolved Hide resolved

XLTechie and others added 3 commits September 18, 2024 21:32

Apply suggestions from code review

10ea475

Co-authored-by: Sascha Cowley <[email protected]>

Update user guide per review suggestion.

78a1398

Add a clarifying comment

1a158f1

XLTechie commented Sep 19, 2024

View reviewed changes

source/gui/__init__.py Outdated Show resolved Hide resolved

XLTechie and others added 2 commits September 18, 2024 21:57

Change a word in the intro message for more clarity.

aba1ce7

Merge branch 'master' into CRFTRefactor

bad035a

Luke Davis added 3 commits September 19, 2024 02:18

Revert "Update user guide per review suggestion.": a newer version ma…

5bcf510

…y be used instead. This reverts commit 78a1398.

Merge branch 'master' into CRFTRefactor

894e820

Potential new user guide entry.

1e58fae

Fix a tiny bit of structure in the UG

f8befd5

Qchristensen approved these changes Sep 19, 2024

View reviewed changes

SaschaCowley merged commit 593d81f into nvaccess:master Sep 20, 2024
4 checks passed

github-actions bot added this to the 2025.1 milestone Sep 20, 2024

coderabbitai bot mentioned this pull request Oct 1, 2024

Update translations from Crowdin #17233

Merged

coderabbitai bot mentioned this pull request Nov 11, 2024

Merge beta to master #17383

Merged

coderabbitai bot mentioned this pull request Dec 9, 2024

Create a MessageDialog API #17304

Merged

31 tasks

coderabbitai bot mentioned this pull request Dec 19, 2024

Fix math not being read in Chromium-based browsers (#17421) #17548

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX #12355

Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX #12355

XLTechie commented Apr 30, 2021 •

edited

Loading

michaelDCurran commented Jul 12, 2021

XLTechie commented Mar 15, 2022 via email

LeonarddeR commented Aug 25, 2023

This comment was marked as outdated.

AppVeyorBot commented Sep 27, 2023

CyrilleB79 commented Jan 25, 2024

This comment was marked as outdated.

CyrilleB79 commented Jan 26, 2024

AppVeyorBot commented Apr 12, 2024

seanbudd commented Sep 2, 2024

XLTechie commented Sep 2, 2024 via email

seanbudd commented Sep 2, 2024

XLTechie commented Sep 19, 2024 •

edited

Loading

SaschaCowley commented Sep 19, 2024

XLTechie commented Sep 19, 2024 •

edited

Loading

Qchristensen left a comment

Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX #12355

Refactor COM Registration Fixing Tool part 2: remove bugs, return Windows errors, improve UX #12355

Conversation

XLTechie commented Apr 30, 2021 • edited Loading

Link to issue number:

Summary of the issue:

User facing changes:

Development details:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Summary by CodeRabbit

michaelDCurran commented Jul 12, 2021

XLTechie commented Mar 15, 2022 via email

LeonarddeR commented Aug 25, 2023

This comment was marked as outdated.

AppVeyorBot commented Sep 27, 2023

CyrilleB79 commented Jan 25, 2024

This comment was marked as outdated.

CyrilleB79 commented Jan 26, 2024

AppVeyorBot commented Apr 12, 2024

seanbudd commented Sep 2, 2024

XLTechie commented Sep 2, 2024 via email

seanbudd commented Sep 2, 2024

XLTechie commented Sep 19, 2024 • edited Loading

SaschaCowley commented Sep 19, 2024

XLTechie commented Sep 19, 2024 • edited Loading

Qchristensen left a comment

Choose a reason for hiding this comment

XLTechie commented Apr 30, 2021 •

edited

Loading

XLTechie commented Sep 19, 2024 •

edited

Loading

XLTechie commented Sep 19, 2024 •

edited

Loading